Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve abs() return type #3263

Merged
merged 4 commits into from
Jul 27, 2024
Merged

Improve abs() return type #3263

merged 4 commits into from
Jul 27, 2024

Conversation

julienfalque
Copy link
Contributor

@phpstan-bot
Copy link
Collaborator

You've opened the pull request against the latest branch 1.12.x. If your code is relevant on 1.11.x and you want it to be released sooner, please rebase your pull request and change its target to 1.11.x.

@julienfalque julienfalque changed the base branch from 1.12.x to 1.11.x July 24, 2024 09:41
ondrejmirtes
ondrejmirtes requested changes Jul 25, 2024
View reviewed changes
Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many instanceof questions about types. This will no doubt be imperfect. The need for such code usually means there should a method on Type instead which will force us to ask the right question in the right places.

Something like Type::getAbsoluteNumericValue(): Type is the way to go.

@julienfalque
Copy link
Contributor Author

julienfalque commented Jul 25, 2024 •
edited
Loading

Added a Type::toAbsoluteNumber() method, this definitely improve things. Let me know if the naming is OK. Also, I'm not certain all implementations are correct.

@julienfalque
Copy link
Contributor Author

By the way, ErrorType inherits from MixedType so:

  • toNumber() returns int|float;
  • toAbsoluteNumber() returns int<0, max>|float.

Shouldn't both methods return ErrorType?

@ondrejmirtes ondrejmirtes merged commit a079c45 into phpstan:1.11.x Jul 27, 2024
447 of 455 checks passed
@ondrejmirtes
Copy link
Member

I love it, thanks!

@ondrejmirtes ondrejmirtes mentioned this pull request Jul 27, 2024
Closed
@julienfalque julienfalque deleted the abs branch July 29, 2024 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@staabm staabm staabm left review comments

@ondrejmirtes ondrejmirtes ondrejmirtes requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

abs() return type ignores specific input ranges
4 participants
@julienfalque @phpstan-bot @ondrejmirtes @staabm